Skip to content

fix(session-replay): Extend masking and focus masking on sensitive information #6292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

fix(session-replay): Extend masking and focus masking on sensitive information #6292

wants to merge 35 commits into from

Conversation

philprime
Copy link
Member

@philprime philprime commented Sep 26, 2025 •
edited
Loading

📜 Description

This PR fixes several session replay masking issues and adds comprehensive test coverage for the redaction builder to prevent regressions.

💡 Motivation and Context

Fixed Masking Issues

SwiftUI.List Background Clipping Issue (#6292)
One screen in the dogfooding app Flinky had significant masking problems where the entire top half of a screen with SwiftUI.List was unmasked. Analysis revealed that _UICollectionViewListLayoutSectionBackgroundColorDecorationView has an extremely large frame (-20 -1135.33; 442 2336) that extends far beyond the visible list bounds, causing incorrect clipping calculations.

Implementation Improvements

String-Based Class Comparison

  • Introduced ExtendedClassIdentifier to compare view classes by string description instead of direct class references
  • Prevents triggering Objective-C +initialize on private UIKit classes when running off the main thread
  • Supports optional layer class filtering for multi-purpose views (e.g., SwiftUI._UIGraphicsView)

Rendering & Layer Improvements

  • Fixed sublayer rendering order by sorting layers by zPosition (with insertion order as tie-breaker)
  • Switched from view-based to layer-based traversal for accurate presentation state
  • Properly handle presentation layers during animations vs model layers

Edge Case Handling

  • UISwitch subtree ignoring to prevent internal UIImageView from being incorrectly redacted
  • UITextField placeholder handling via UITextFieldLabel detection
  • SwiftUI.List background decoration view treated as special case (direct redact, no clip-out)
  • Axis-aligned transform detection to optimize opaque view clipping
  • Custom anchor point support in transform calculations

Test Coverage

Added 49 new tests to systematically verify:

  • ExtendedClassIdentifier matching with layer filtering
  • UIImageView boundary conditions (nil, 9x9, 10x10, 11x11 pixels)
  • Hidden views and zero opacity handling
  • Transform calculations with custom anchor points
  • Region ordering and SwiftUI region prioritization
  • Nested clipping regions
  • Class hierarchy inheritance patterns
  • iOS version-specific behavior (iOS 26 UISlider visual element)

Note: SwiftUI-specific integration tests (Text, Image, List, Label, Button) will be added in a follow-up PR.

💚 How did you test it?

Manual Testing

  • Verified fix in Flinky app - SwiftUI.List screens now mask correctly
  • Tested across iOS 15.5, 16.4, 17.5, 18.6, and 26.0 simulators

Automated Testing

  • 49 new test cases added
  • All 118+ tests passing (0 failures)
  • Tests follow Arrange-Act-Assert pattern
  • Standardized snapshot naming (masked/unmasked)

Files Changed

  • Sources/Swift/Core/Tools/ViewCapture/SentryUIRedactBuilder.swift - Core fixes
  • Tests/SentryTests/ViewCapture/SentryUIRedactBuilderTests+EdgeCases.swift - New (32 tests)
  • Tests/SentryTests/ViewCapture/SentryUIRedactBuilderTests+Common.swift - Enhanced (8 new tests)
  • Tests/SentryTests/ViewCapture/SentryUIRedactBuilderTests+UIKit.swift - Enhanced (5 new tests, removed 2 duplicates)
  • Tests/SentryTests/ViewCapture/SentryUIRedactBuilderTests+SpecialViews.swift - Updated naming

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

@philprime philprime self-assigned this Sep 26, 2025
@codecov Codecov
Copy link

codecov bot commented Sep 26, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 94.66667% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.444%. Comparing base (caa7e70) to head (05f3e6a).
⚠️ Report is 24 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...Core/Tools/ViewCapture/SentryUIRedactBuilder.swift 96.551% 7 Missing ⚠️
...ft/Core/Tools/ViewCapture/SentryRedactRegion.swift 72.222% 5 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #6292       +/-   ##
=============================================
+ Coverage   86.865%   87.444%   +0.578%     
=============================================
  Files          445       450        +5     
  Lines        37505     37751      +246     
  Branches     17440     17428       -12     
=============================================
+ Hits         32579     33011      +432     
+ Misses        4880      4697      -183     
+ Partials        46        43        -3
Files with missing lines Coverage Δ
...ore/Tools/ViewCapture/SentryViewPhotographer.swift 76.000% <100.000%> (+2.086%) ⬆️
...ft/Core/Tools/ViewCapture/SentryRedactRegion.swift 82.142% <72.222%> (-17.858%) ⬇️
...Core/Tools/ViewCapture/SentryUIRedactBuilder.swift 95.052% <96.551%> (-0.221%) ⬇️

... and 88 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update caa7e70...05f3e6a. Read the comment docs.

@philprime philprime mentioned this pull request Sep 26, 2025
Open
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 26, 2025 •
edited
Loading

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1190.59 ms 1244.60 ms 54.01 ms
Size 23.75 KiB 1.00 MiB 1006.18 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
aff3b66 1229.53 ms 1263.08 ms 33.55 ms
570f725 1206.00 ms 1238.96 ms 32.96 ms
2481950 1221.04 ms 1248.98 ms 27.94 ms
d3f650a 1225.45 ms 1241.86 ms 16.41 ms
07f6759 1232.98 ms 1267.59 ms 34.61 ms
1fecbb8 1242.78 ms 1265.40 ms 22.62 ms
6e99155 1223.96 ms 1249.25 ms 25.29 ms
fc05805 1220.63 ms 1252.16 ms 31.54 ms
0e9c5ae 1226.10 ms 1254.14 ms 28.04 ms
397b9c9 1230.23 ms 1249.29 ms 19.06 ms

App size

Revision Plain With Sentry Diff
aff3b66 23.75 KiB 978.53 KiB 954.78 KiB
570f725 23.74 KiB 913.38 KiB 889.63 KiB
2481950 23.74 KiB 872.74 KiB 849.00 KiB
d3f650a 23.75 KiB 902.48 KiB 878.73 KiB
07f6759 23.75 KiB 997.26 KiB 973.52 KiB
1fecbb8 23.75 KiB 969.28 KiB 945.53 KiB
6e99155 23.75 KiB 963.18 KiB 939.43 KiB
fc05805 23.75 KiB 908.02 KiB 884.27 KiB
0e9c5ae 23.75 KiB 969.29 KiB 945.54 KiB
397b9c9 23.75 KiB 959.44 KiB 935.70 KiB

Previous results on branch: philprime/fix-masking

Startup times

Revision Plain With Sentry Diff
bb49b7a 1229.51 ms 1260.31 ms 30.80 ms
d70d220 1238.80 ms 1262.08 ms 23.29 ms
5a4767b 1206.00 ms 1238.21 ms 32.21 ms
8d1f930 1241.27 ms 1263.71 ms 22.44 ms

App size

Revision Plain With Sentry Diff
bb49b7a 23.75 KiB 1014.06 KiB 990.32 KiB
d70d220 23.75 KiB 994.81 KiB 971.06 KiB
5a4767b 23.75 KiB 1.01 MiB 1006.46 KiB
8d1f930 23.75 KiB 981.78 KiB 958.03 KiB

@philprime philprime changed the title fix(session-replay): Ignore list background decoration view in redaction fix(session-replay): Extend masking and focus masking on sensitive information Oct 2, 2025
@philprime philprime mentioned this pull request Oct 6, 2025
Merged
7 tasks
@philprime philprime marked this pull request as ready for review October 8, 2025 12:26
@philprime philprime marked this pull request as draft October 8, 2025 12:27
cursor[bot]

This comment was marked as outdated.

Sign in to view

seer-by-sentry[bot]
seer-by-sentry bot reviewed Oct 8, 2025
View reviewed changes
This was referenced Oct 9, 2025
Merged
Open
@philprime philprime mentioned this pull request Oct 15, 2025
Draft
7 tasks
@philprime philprime marked this pull request as ready for review October 21, 2025 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seer-by-sentry seer-by-sentry[bot] seer-by-sentry[bot] left review comments

@cursor cursor[bot] cursor[bot] left review comments

@philipphofmann philipphofmann Awaiting requested review from philipphofmann philipphofmann is a code owner

@noahsmartin noahsmartin Awaiting requested review from noahsmartin noahsmartin is a code owner

@itaybre itaybre Awaiting requested review from itaybre itaybre is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@philprime philprime

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@philprime